Skip to content

Core: Preserve empty partition bounds in inspect.manifests#3708

Open
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/inspect-manifests-empty-bounds
Open

Core: Preserve empty partition bounds in inspect.manifests#3708
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:fix/inspect-manifests-empty-bounds

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Manifest partition summaries encode string bounds as bytes. An empty string is represented by b"", but InspectTable.manifests() checked encoded bounds by truthiness. As a result, valid empty-string lower and upper bounds were returned as null.

This change checks whether each bound is absent instead, preserving empty values while continuing to return null for missing bounds. This matches Apache Iceberg Java's manifests metadata table.

Are these changes tested?

Yes. A regression test writes a table partitioned by an identity-transformed string containing an empty value and verifies that both manifest partition bounds remain empty strings.

The following checks pass locally:

  • PYTHONPATH=. uv run pytest tests/table/test_inspect.py -q (5 passed)
  • PYTHONPATH=. uv run --extra datafusion --extra pyiceberg-core pytest -p no:cacheprovider tests/table -q (328 passed)
  • make lint

Are there any user-facing changes?

Yes. table.inspect.manifests() now reports empty-string partition bounds as empty strings instead of null. There are no API changes.

@fallintoplace fallintoplace changed the title Core: Preserve empty bounds in inspect.manifests Core: Preserve empty partition bounds in inspect.manifests Jul 25, 2026
Comment thread tests/table/test_inspect.py Outdated


def test_inspect_manifests_preserves_empty_string_bounds(catalog: InMemoryCatalog) -> None:
schema = Schema(NestedField(1, "s", StringType(), required=False))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required=False looks redundant.

Comment thread tests/table/test_inspect.py Outdated
schema = Schema(NestedField(1, "s", StringType(), required=False))
spec = PartitionSpec(PartitionField(1, 1000, IdentityTransform(), "s"))
tbl = catalog.create_table("default.empty_string_partition", schema, partition_spec=spec)
tbl.append(pa.table({"s": [""]}, schema=pa.schema([pa.field("s", pa.large_string(), nullable=True)])))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullable=True looks redundant.

@fallintoplace
fallintoplace force-pushed the fix/inspect-manifests-empty-bounds branch from 32e09ad to b0ab723 Compare July 27, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants